Skip to content

Add redirect URL caching for ACL and certificate service clients - #46254

Merged
ryazhang-microsoft merged 12 commits into
mainfrom
ryazhang/redirect-url-caching
Apr 27, 2026
Merged

Add redirect URL caching for ACL and certificate service clients#46254
ryazhang-microsoft merged 12 commits into
mainfrom
ryazhang/redirect-url-caching

Conversation

@ryazhang-microsoft

@ryazhang-microsoft ryazhang-microsoft commented Apr 10, 2026

Copy link
Copy Markdown
Member

What

Add a custom RedirectCachingPolicy that caches redirect target URLs for write operations (POST/PUT/PATCH/DELETE), so subsequent writes skip the load balancer and go directly to the primary node.

Why

Write requests to ACL are redirected (307) from the load balancer to the primary node. Without caching, every write incurs a redirect round-trip (~2000ms). With caching, only the first write is redirected; subsequent writes go direct (~20ms).

Behavior

Scenario Behavior
Write (POST/PUT/PATCH/DELETE) Uses cached primary URL if warm, caches on first 307
Read (GET) Always goes through load balancer, never uses cache
5xx on write Invalidates cache
Transport error on write Invalidates cache

Changes

  • New _redirect_caching_policy.py in both azure-confidentialledger and azure-confidentialledger-certificate
  • Replace default RedirectPolicy with RedirectCachingPolicy / AsyncRedirectCachingPolicy in all 4 client files
  • Fix missing disable_redirect_cleanup=True in async ACL client
  • Add disable_redirect_cleanup=True to certificate service clients
image

Write requests (POST/PUT/PATCH/DELETE) that receive a 307 redirect from
the load balancer cache the target URL so subsequent writes skip the LB.
GET requests always go through the LB and never use the cache.

The cache is invalidated on 5xx responses or transport errors to handle
failover scenarios.

Changes:
- Add RedirectCachingPolicy and AsyncRedirectCachingPolicy to both
  azure-confidentialledger and azure-confidentialledger-certificate
- Replace default RedirectPolicy with RedirectCachingPolicy in all
  4 client files (sync/async for both packages)
- Fix missing disable_redirect_cleanup=True in async ACL client
- Add disable_redirect_cleanup=True to certificate service clients

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a custom redirect policy for Confidential Ledger clients that caches the primary-node redirect target for write operations, so subsequent writes avoid repeated 307 redirect round-trips via the load balancer.

Changes:

  • Added custom redirect-caching policies (sync + async) for ACL and certificate service packages.
  • Updated all four generated clients to use the redirect-caching policies in the pipeline.
  • Updated SensitiveHeaderCleanupPolicy usage to disable redirect cleanup in more clients.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/aio/_client.py Swaps in async redirect-caching policy and adjusts sensitive header cleanup policy construction.
sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_redirect_caching_policy.py Adds the core redirect-caching policy implementation (sync + async).
sdk/confidentialledger/azure-confidentialledger/azure/confidentialledger/_client.py Swaps in sync redirect-caching policy in the generated client pipeline.
sdk/confidentialledger/azure-confidentialledger-certificate/azure/confidentialledger/certificate/aio/_client.py Swaps in async redirect-caching policy and changes sensitive header cleanup behavior.
sdk/confidentialledger/azure-confidentialledger-certificate/azure/confidentialledger/certificate/_redirect_caching_policy.py Adds the certificate package’s redirect-caching policy implementation (sync + async).
sdk/confidentialledger/azure-confidentialledger-certificate/azure/confidentialledger/certificate/_client.py Swaps in sync redirect-caching policy and changes sensitive header cleanup behavior.

@ryazhang-microsoft
ryazhang-microsoft enabled auto-merge (squash) April 10, 2026 18:26
Comment thread sdk/confidentialledger/azure-confidentialledger-certificate/CHANGELOG.md Outdated
Comment thread sdk/confidentialledger/azure-confidentialledger-certificate/CHANGELOG.md Outdated
Comment thread sdk/confidentialledger/azure-confidentialledger/CHANGELOG.md
Tewatia5355 pushed a commit to Azure/azure-sdk-for-js that referenced this pull request Apr 23, 2026
Write requests (POST/PUT/PATCH/DELETE) that receive a 307/308 redirect now
cache the target URL's base (scheme + host) so subsequent writes skip the
load balancer, reducing latency. Read requests (GET/HEAD/OPTIONS) always
go through the load balancer and never use or populate the cache.

Cache is automatically invalidated on 5xx responses or transport errors.
Original URL is saved and restored on errors for clean retry behavior.

Also adds support for HTTP 308 (Permanent Redirect) in the redirect policy.

Mirrors the Python SDK's RedirectCachingPolicy (Azure/azure-sdk-for-python#46254).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tewatia5355 pushed a commit to Azure/azure-sdk-for-js that referenced this pull request Apr 27, 2026
Write requests (POST/PUT/PATCH/DELETE) that receive a 307/308 redirect now
cache the target URL's base (scheme + host) so subsequent writes skip the
load balancer, reducing latency. Read requests (GET/HEAD/OPTIONS) always
go through the load balancer and never use or populate the cache.

Cache is automatically invalidated on 5xx responses or transport errors.
Original URL is saved and restored on errors for clean retry behavior.

Also adds support for HTTP 308 (Permanent Redirect) in the redirect policy.

Mirrors the Python SDK's RedirectCachingPolicy (Azure/azure-sdk-for-python#46254).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ryazhang-microsoft
ryazhang-microsoft merged commit 3c9162c into main Apr 27, 2026
19 checks passed
@ryazhang-microsoft
ryazhang-microsoft deleted the ryazhang/redirect-url-caching branch April 27, 2026 20:25
Tewatia5355 pushed a commit to Azure/azure-sdk-for-js that referenced this pull request Apr 29, 2026
Write requests (POST/PUT/PATCH/DELETE) that receive a 307/308 redirect now
cache the target URL's base (scheme + host) so subsequent writes skip the
load balancer, reducing latency. Read requests (GET/HEAD/OPTIONS) always
go through the load balancer and never use or populate the cache.

Cache is automatically invalidated on 5xx responses or transport errors.
Original URL is saved and restored on errors for clean retry behavior.

Also adds support for HTTP 308 (Permanent Redirect) in the redirect policy.

Mirrors the Python SDK's RedirectCachingPolicy (Azure/azure-sdk-for-python#46254).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tewatia5355 added a commit to Azure/azure-sdk-for-js that referenced this pull request Apr 30, 2026
…ons (#38259)

### Packages impacted by this PR

`@azure-rest/confidential-ledger`

### Issues associated with this PR

Related to [ADO Task
#37485929](https://msazure.visualstudio.com/One/_workitems/edit/37485929)
(parent Feature #37442109)

### Describe the problem that is addressed by this PR

When writing to Azure Confidential Ledger, the load balancer redirects
write requests (POST/PUT/PATCH/DELETE) to the primary node via a 307
redirect. Every write pays this redirect cost, adding 3-10 seconds of
latency per request. This is especially painful for batch workloads that
do many sequential writes.

This PR caches the redirect target URL after the first 307/308 redirect
so that subsequent writes go directly to the primary node, bringing
write latency down to around 230ms.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The design mirrors Ryan Zhang's Python SDK implementation (PR #46254 in
azure-sdk-for-python). A closure-based cache variable holds the redirect
target URL within the existing confidentialLedgerRedirectPolicy. This
was chosen because:

- It keeps cache state per-client (no shared global state between
clients)
- It builds on the existing custom redirect policy added in PR #37249
- It matches the behavior of the Python SDK so users get a consistent
experience across languages

Cache behavior:
- Write requests (POST/PUT/PATCH/DELETE) that receive a 307/308 cache
the target and reuse it on subsequent writes
- Read requests (GET/HEAD) never use or populate the cache
- Server errors (5xx) or transport errors on writes invalidate the cache
so the next write goes back through the load balancer
- 303 POST-to-GET conversions do not cache (the method changed, so the
target is not guaranteed to be the primary)

### Are there test cases added in this PR?

Yes. 18 new unit tests covering cache population, cache bypass for
reads, cache invalidation on errors, and edge cases (308 support, URL
restore on transport errors, 303 conversion handling, redirect-follow
failures). All 30 tests pass (12 existing + 18 new).

Also live tested against a real ledger (ryan-sdk-acl). Round 1 showed
the expected 307 redirect on the first write, and Round 2 showed 0
redirects with 5 direct writes at around 230ms each.

### Provide a list of related PRs

- Ryan Zhang's auth-preservation PR (merged): #37249
- Ryan Zhang's Python SDK caching (reference implementation):
[azure-sdk-for-python#46254](Azure/azure-sdk-for-python#46254)

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator? No
- [x] Added a changelog (if necessary)

Co-authored-by: Yash Kumar <yakumar@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tewatia5355 added a commit to Azure/azure-sdk-for-js that referenced this pull request May 19, 2026
#38569)

### Packages impacted by this PR

`@azure-rest/confidential-ledger` - bumps `1.1.2-beta.5` (unreleased) →
`1.1.2-beta.6`.

### Issues associated with this PR

- MSRC #116673 (IcM #31000000604457)
- ADO Task #37485929 (parent Feature #37442109)

> ⚠️ Coordinated disclosure: CHANGELOG wording intentionally generic,
please review with the MSRC case manager (Abhilash Konnur) and
@ryazhang-microsoft before exiting draft.

### Describe the problem that is addressed by this PR

The custom `confidentialLedgerRedirectPolicy` shipped in `1.1.2-beta.4`
/ `beta.5` replaces the default `redirectPolicy` and preserves the
`Authorization` header on redirects (required because Confidential
Ledger redirects the LB → primary node on a subdomain). It did **not**
verify the redirect target stayed within the ledger's trust boundary, so
a compromised/attacker-controlled initial endpoint could 307/308 to
`http://attacker/...` and the SDK would forward the Bearer token + write
body. The caching added in #38259 worsens it by persistently rewriting
subsequent writes to the malicious target.

This PR introduces a trust check evaluated **before** any mutation of
the request URL/method: target must be HTTPS, port must match the
configured ledger port, and the canonical hostname must equal the ledger
hostname or be a subdomain of it (leading-dot boundary). Untrusted
redirects throw a non-retryable error, so neither credentials nor the
request body reach the untrusted host and the cache cannot be poisoned.

### What are the possible designs available to address the problem?

1. **Strict same-origin (MSRC literal recommendation)** - *rejected*:
would break legitimate LB → subdomain primary redirects.
2. **Strip-and-follow on cross-origin** - *rejected*: still leaks the
request body to the untrusted host.
3. **Subdomain-allowed trust check + refuse on mismatch (chosen, agreed
with @ryazhang-microsoft)** - fixes the vuln without breaking the
legitimate redirect pattern; stronger than MSRC literal because it also
prevents body leak.

Implementation notes:
- Trust anchor (`ledgerHostname`, `ledgerPort`) captured at policy
creation. Hostnames canonicalized (lowercase + trailing dot stripped);
subdomain check uses leading-dot boundary so prefix-collisions
(`evil-test-ledger.<...>`) are rejected.
- Cache writes **staged** during redirect chain; only committed after a
successful non-5xx trusted chain. Discarded on throw / transport error /
5xx / untrusted hop.
- Outer `sendRequest` `catch` restores original URL/method and
invalidates cache for writes - clean retry semantics.

### Are there test cases added in this PR?

Yes. **48/48 unit tests pass.**

- 12 legacy redirect tests (fixtures updated to subdomain pattern).
- 14 caching tests (already on subdomain pattern, untouched).
- **22 new security regression tests** in
`confidentialLedgerRedirectTrust.spec.ts`: cross-origin attacker host,
HTTP downgrade (matches MSRC PoC), sibling domain, different port,
prefix-collision (`evil-test-ledger.*` vs `test-ledger.*`),
protocol-relative `Location`, 301/302/303 untrusted, trusted subdomain
follow, trailing-dot FQDN, uppercase host, mid-chain untrusted hop with
staged-cache discard, warm-cache invalidation when the cached node
returns an untrusted redirect, and a **full-pipeline integration test**
that goes through `createClient` with a mock `HttpClient` and asserts
the Bearer token never reaches `attacker.example.com`.

**Mutation tested:** temporarily disabling the throw on untrusted
redirect → 14 security tests correctly fail (proves tests test behavior,
not implementation).

**Live-tested** against `ryan-sdk-acl`: 3 writes succeeded (cold cache
243 ms, warm cache 231/245 ms), 0 false-positive trust warnings.

### Provide a list of related PRs

- #38259 - redirect URL caching (this PR makes that caching safe).
- #37249 - original custom redirect policy (vulnerable since
`1.1.2-beta.4`).
- Azure/azure-sdk-for-python#46254 - Python caching PR. Matching Python
security fix is on a separate azure-core-driven path per
@ryazhang-microsoft.

### Command used to generate this PR

N/A - security fix to a hand-authored customization, not a release
request.

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR need any fixes in the SDK Generator? - **N/A**,
change is in `src/confidentialLedger.ts` (hand-authored customization),
not generated code.
- [x] Added a changelog (if necessary)

---------

Co-authored-by: Yash Kumar <yakumar@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants